Skip to content

build: allow no-remote-exec targets to be cached on CI - #23789

Merged
clydin merged 2 commits into
angular:mainfrom
jbedard:bazel-test-cache
Aug 29, 2022
Merged

build: allow no-remote-exec targets to be cached on CI#23789
clydin merged 2 commits into
angular:mainfrom
jbedard:bazel-test-cache

Conversation

@jbedard

@jbedard jbedard commented Aug 24, 2022

Copy link
Copy Markdown
Contributor

No description provided.

…nalysis cache

Use renamed experimental_allow_tags_propagation instead of previous name (incompatible_allow_tags_propagation).
Comment thread .bazelrc
Comment thread .bazelrc
@jbedard jbedard changed the title test: fix bazel caching of unit tests build: allow no-remote-exec targets to be cached on CI Aug 24, 2022
sudo bash -c "echo -e 'build --google_credentials=/home/circleci/.gcp_credentials' >> .bazelrc.user";
# Upload/don't upload local results to cache based on environment
if [[ -n "{$CIRCLE_PULL_REQUEST}" ]]; then
sudo bash -c "echo -e 'build:remote --remote_upload_local_results=false\n' >> .bazelrc.user";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this done? Disabling remote caching for targets where remote execution is disabled...

Normally this would only be done for things like local dev to read but not write to the remote cache. I thought maybe it was to avoid polluting the cache server with random PRs but most bazel targets (all but 3 or 4) get executed remote and are cached despite this flag.

But this is what was causing a few of the unit tests and all the e2e tests (e2e being a WIP) to never be cached.

@jbedard
jbedard marked this pull request as ready for review August 25, 2022 03:08
@jbedard
jbedard requested a review from alan-agius4 August 25, 2022 03:08
@alan-agius4
alan-agius4 requested review from josephperrott and removed request for alan-agius4 August 25, 2022 08:13

@josephperrott josephperrott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Aug 25, 2022

@devversion devversion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one comment about the legacy external runfiles

Comment thread .bazelrc
Comment thread .bazelrc
@clydin
clydin merged commit 1cc2d65 into angular:main Aug 29, 2022
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants